-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Revert "[NFCI][Globals] In GlobalObjects::setSectionPrefix, do conditional update if existing prefix is not equivalent to the new one. Returns whether prefix changed." #159159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ional up…" This reverts commit 027bccc.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: Mingming Liu (mingmingl-llvm) ChangesReverts llvm/llvm-project#158460 due to buildbot failures Full diff: https://github.com/llvm/llvm-project/pull/159159.diff 7 Files Affected:
diff --git a/llvm/include/llvm/IR/GlobalObject.h b/llvm/include/llvm/IR/GlobalObject.h
index e273387807cf6..08a02b42bdc14 100644
--- a/llvm/include/llvm/IR/GlobalObject.h
+++ b/llvm/include/llvm/IR/GlobalObject.h
@@ -121,10 +121,8 @@ class GlobalObject : public GlobalValue {
/// appropriate default object file section.
LLVM_ABI void setSection(StringRef S);
- /// If existing prefix is different from \p Prefix, set it to \p Prefix. If \p
- /// Prefix is empty, the set clears the existing metadata. Returns true if
- /// section prefix changed and false otherwise.
- LLVM_ABI bool setSectionPrefix(StringRef Prefix);
+ /// Set the section prefix for this global object.
+ LLVM_ABI void setSectionPrefix(StringRef Prefix);
/// Get the section prefix for this global object.
LLVM_ABI std::optional<StringRef> getSectionPrefix() const;
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 92d87681c9adc..9db4c9e5e2807 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -583,23 +583,23 @@ bool CodeGenPrepare::_run(Function &F) {
// if requested.
if (BBSectionsGuidedSectionPrefix && BBSectionsProfileReader &&
BBSectionsProfileReader->isFunctionHot(F.getName())) {
- EverMadeChange |= F.setSectionPrefix("hot");
+ F.setSectionPrefix("hot");
} else if (ProfileGuidedSectionPrefix) {
// The hot attribute overwrites profile count based hotness while profile
// counts based hotness overwrite the cold attribute.
// This is a conservative behabvior.
if (F.hasFnAttribute(Attribute::Hot) ||
PSI->isFunctionHotInCallGraph(&F, *BFI))
- EverMadeChange |= F.setSectionPrefix("hot");
+ F.setSectionPrefix("hot");
// If PSI shows this function is not hot, we will placed the function
// into unlikely section if (1) PSI shows this is a cold function, or
// (2) the function has a attribute of cold.
else if (PSI->isFunctionColdInCallGraph(&F, *BFI) ||
F.hasFnAttribute(Attribute::Cold))
- EverMadeChange |= F.setSectionPrefix("unlikely");
+ F.setSectionPrefix("unlikely");
else if (ProfileUnknownInSpecialSection && PSI->hasPartialSampleProfile() &&
PSI->isFunctionHotnessUnknown(F))
- EverMadeChange |= F.setSectionPrefix("unknown");
+ F.setSectionPrefix("unknown");
}
/// This optimization identifies DIV instructions that can be
diff --git a/llvm/lib/CodeGen/StaticDataAnnotator.cpp b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
index 53a9ab4dbda02..2d9b489a80acb 100644
--- a/llvm/lib/CodeGen/StaticDataAnnotator.cpp
+++ b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
@@ -91,7 +91,8 @@ bool StaticDataAnnotator::runOnModule(Module &M) {
if (SectionPrefix.empty())
continue;
- Changed |= GV.setSectionPrefix(SectionPrefix);
+ GV.setSectionPrefix(SectionPrefix);
+ Changed = true;
}
return Changed;
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 1a7a5c5fbad6b..11d33e262fecb 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -288,22 +288,10 @@ void GlobalObject::setSection(StringRef S) {
setGlobalObjectFlag(HasSectionHashEntryBit, !S.empty());
}
-bool GlobalObject::setSectionPrefix(StringRef Prefix) {
- StringRef ExistingPrefix;
- if (std::optional<StringRef> MaybePrefix = getSectionPrefix())
- ExistingPrefix = *MaybePrefix;
-
- if (ExistingPrefix == Prefix)
- return false;
-
- if (Prefix.empty()) {
- setMetadata(LLVMContext::MD_section_prefix, nullptr);
- return true;
- }
+void GlobalObject::setSectionPrefix(StringRef Prefix) {
MDBuilder MDB(getContext());
setMetadata(LLVMContext::MD_section_prefix,
MDB.createGlobalObjectSectionPrefix(Prefix));
- return true;
}
std::optional<StringRef> GlobalObject::getSectionPrefix() const {
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
index c86092bd51eda..ecb2f2dbc552b 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
@@ -848,12 +848,13 @@ bool MemProfUsePass::annotateGlobalVariables(
// So we just print out the static data section prefix in LLVM_DEBUG.
if (Record && Record->AccessCount > 0) {
++NumOfMemProfHotGlobalVars;
- Changed |= GVar.setSectionPrefix("hot");
+ GVar.setSectionPrefix("hot");
+ Changed = true;
LLVM_DEBUG(dbgs() << "Global variable " << Name
<< " is annotated as hot\n");
} else if (DataAccessProf->isKnownColdSymbol(Name)) {
++NumOfMemProfColdGlobalVars;
- Changed |= GVar.setSectionPrefix("unlikely");
+ GVar.setSectionPrefix("unlikely");
Changed = true;
LLVM_DEBUG(dbgs() << "Global variable " << Name
<< " is annotated as unlikely\n");
diff --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt
index d62ce66ef9d34..8b7bd3997ea27 100644
--- a/llvm/unittests/IR/CMakeLists.txt
+++ b/llvm/unittests/IR/CMakeLists.txt
@@ -28,7 +28,6 @@ add_llvm_unittest(IRTests
DominatorTreeBatchUpdatesTest.cpp
DroppedVariableStatsIRTest.cpp
FunctionTest.cpp
- GlobalObjectTest.cpp
PassBuilderCallbacksTest.cpp
IRBuilderTest.cpp
InstructionsTest.cpp
diff --git a/llvm/unittests/IR/GlobalObjectTest.cpp b/llvm/unittests/IR/GlobalObjectTest.cpp
deleted file mode 100644
index 0e16d01e759de..0000000000000
--- a/llvm/unittests/IR/GlobalObjectTest.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-//===- GlobalObjectTest.cpp - Global object unit tests --------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/IR/GlobalObject.h"
-#include "llvm/AsmParser/Parser.h"
-#include "llvm/IR/Module.h"
-#include "llvm/Support/SourceMgr.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-using namespace llvm;
-namespace {
-using testing::Eq;
-using testing::Optional;
-using testing::StrEq;
-
-static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
- SMDiagnostic Err;
- std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
- if (!Mod)
- Err.print("GlobalObjectTests", errs());
- return Mod;
-}
-
-static LLVMContext C;
-static std::unique_ptr<Module> M;
-
-class GlobalObjectTest : public testing::Test {
-public:
- static void SetUpTestSuite() {
- M = parseIR(C, R"(
-@foo = global i32 3, !section_prefix !0
-@bar = global i32 0
-
-!0 = !{!"section_prefix", !"hot"}
-)");
- }
-};
-
-TEST_F(GlobalObjectTest, SectionPrefix) {
- GlobalVariable *Foo = M->getGlobalVariable("foo");
-
- // Initial section prefix is hot.
- ASSERT_NE(Foo, nullptr);
- ASSERT_THAT(Foo->getSectionPrefix(), Optional(StrEq("hot")));
-
- // Test that set method returns false since existing section prefix is hot.
- EXPECT_FALSE(Foo->setSectionPrefix("hot"));
-
- // Set prefix from hot to unlikely.
- Foo->setSectionPrefix("unlikely");
- EXPECT_THAT(Foo->getSectionPrefix(), Optional(StrEq("unlikely")));
-
- // Set prefix to empty is the same as clear.
- Foo->setSectionPrefix("");
- // Test that section prefix is cleared.
- EXPECT_THAT(Foo->getSectionPrefix(), Eq(std::nullopt));
-
- GlobalVariable *Bar = M->getGlobalVariable("bar");
-
- // Initial section prefix is empty.
- ASSERT_NE(Bar, nullptr);
- ASSERT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
-
- // Test that set method returns false since Bar doesn't have prefix metadata.
- EXPECT_FALSE(Bar->setSectionPrefix(""));
-
- // Set from empty to hot.
- EXPECT_TRUE(Bar->setSectionPrefix("hot"));
- EXPECT_THAT(Bar->getSectionPrefix(), Optional(StrEq("hot")));
-
- // Test that set method returns true and section prefix is cleared.
- EXPECT_TRUE(Bar->setSectionPrefix(""));
- EXPECT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
-}
-} // namespace
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/14677 Here is the relevant piece of the build log for the reference |
Reverts #158460 due to buildbot failures